bin/warp-graph.js (1)

25-25: Prefer a public API over mutating graph._seekCache directly.
Relying on a private field risks breakage if internals change; consider wiring via WarpGraph.open({ seekCache }) or a dedicated setter.

Also applies to: 1946-1955

test/unit/domain/seekCache.test.js (1)

196-212: Tests are coupled to private fields _cachedState, _cachedCeiling, _cachedFrontier.

Nullifying internal cache fields (Lines 201–203, repeated in several tests) to force the persistent cache path is fragile — any rename or structural change to WarpGraph internals will break these tests silently. Consider exposing a clearInMemoryCache() or _resetCeilingCache() test helper on WarpGraph to decouple tests from field-level internals.

src/infrastructure/adapters/CasSeekCacheAdapter.js (2)

50-58: maxEntries of 0 silently defaults to 200.

maxEntries || DEFAULT_MAX_ENTRIES treats 0 as falsy. While 0 entries would be impractical, if a caller explicitly passes 0 intending "disabled", they'd get 200 entries instead. Use nullish coalescing for clarity.

Proposed fix

-    this._maxEntries = maxEntries || DEFAULT_MAX_ENTRIES;
+    this._maxEntries = maxEntries ?? DEFAULT_MAX_ENTRIES;

---

261-268: clear() only deletes the ref — orphaned CAS trees/blobs remain.

This is consistent with the module doc ("blobs are loose Git objects — git gc prunes them naturally"), but callers may expect clear() to reclaim space immediately. Worth a brief JSDoc note on clear() that CAS blobs are left for git gc.

src/domain/WarpGraph.js (1)

865-874: Misleading comment: "fire-and-forget" but the set() is awaited.

The await on Line 870 blocks return until the cache write completes (or fails). The comment at Line 865 says "fire-and-forget" which typically implies not awaiting. Since failure is caught and swallowed, the behavior is correct but the term is misleading. Either drop the await to make it truly fire-and-forget, or fix the comment.

Additionally, the cacheKey is computed identically on Line 811 and Line 867. Hoist it above the persistent-cache-check block to avoid the redundant SHA-256 computation on cache-miss-then-store.

♻️ Proposed fix: hoist cacheKey and fix comment

     // Persistent cache check — skip when collectReceipts is requested
+    let cacheKey;
     if (this._seekCache && !collectReceipts) {
-      const cacheKey = buildSeekCacheKey(ceiling, frontier);
+      cacheKey = buildSeekCacheKey(ceiling, frontier);
       try {
         const cached = await this._seekCache.get(cacheKey);

-    // Store to persistent cache (fire-and-forget for non-receipt paths)
+    // Store to persistent cache (failure is non-fatal)
     if (this._seekCache && !collectReceipts && allPatches.length > 0) {
-      const cacheKey = buildSeekCacheKey(ceiling, frontier);
       try {
+        if (!cacheKey) cacheKey = buildSeekCacheKey(ceiling, frontier);
         const buf = serializeFullStateV5(state, { codec: this._codec });
         await this._seekCache.set(cacheKey, buf);

src/infrastructure/adapters/CasSeekCacheAdapter.js (1)

185-192: Consider guarding against malformed cache keys.

A malformed key yields NaN ceiling and an empty hash; a small validation makes failures explicit.

🔧 Suggested guard

   const dashIdx = rest.indexOf('-');
-  const ceiling = parseInt(rest.slice(1, dashIdx), 10);
+  if (!rest.startsWith('t') || dashIdx <= 1) {
+    throw new Error(`Invalid seek cache key: ${key}`);
+  }
+  const ceiling = Number.parseInt(rest.slice(1, dashIdx), 10);
+  if (!Number.isInteger(ceiling)) {
+    throw new Error(`Invalid seek cache key: ${key}`);
+  }
   const frontierHash = rest.slice(dashIdx + 1);
+  if (!frontierHash) {
+    throw new Error(`Invalid seek cache key: ${key}`);
+  }

test/unit/infrastructure/adapters/GitGraphAdapter.coverage.test.js (1)

115-117: Unused mockStream variable.

mockStream is declared but never referenced — the executeStream mock on lines 120–127 creates its own response objects directly. This dead code can be removed.

🧹 Suggested cleanup

-      const mockStream = {
-        collect: vi.fn(),
-      };
-      // First call returns content for file_a, second for file_b

src/domain/services/BitmapIndexBuilder.js (1)

19-22: computeChecksum is duplicated across three bitmap service files.

Identical implementations exist in both BitmapIndexBuilder.js and StreamingBitmapIndexBuilder.js (same pattern: canonical stringify → SHA-256 via crypto). A similar function also appears in BitmapIndexReader.js with an additional version parameter and conditional stringify logic. Consider extracting the base checksum pattern to a shared utility to keep implementations in sync and reduce duplication.

src/domain/services/StateSerializerV5.js (1)

125-133: JSDoc crypto param should be marked optional.

Since computeStateHashV5 now falls back to defaultCrypto, the crypto parameter is effectively optional. The JSDoc on line 126 should use bracket syntax to reflect this.

📝 Suggested fix

- * `@param` {import('../../ports/CryptoPort.js').default} options.crypto - CryptoPort instance
+ * `@param` {import('../../ports/CryptoPort.js').default} [options.crypto] - CryptoPort instance (defaults to defaultCrypto)

test/unit/domain/WarpGraph.coverageGaps.test.js (1)

280-309: Consider asserting GC result shape more specifically.

The "runs GC when tombstone ratio threshold is exceeded" test (line 280) only checks result.ran, result.result, and result.reasons.length. Since the state has a live node and zero tombstones, it may be worth asserting result.result.tombstonesRemoved === 0 to confirm the GC ran but found nothing to compact — that would make the test intent clearer and catch regressions in the GC result shape.

src/domain/WarpGraph.js (1)

951-962: Correct semantic fix — but orsetElements() is O(n) per call.

Switching from .size (which counts raw entries including tombstones) to orsetElements().length (which counts only live elements) is the correct fix for accurate before/after counts. However, orsetElements() iterates the entire ORSet and filters by containment — it's called 4 times in join() (twice for nodes, twice for edges).

For large graphs this could be noticeable. Consider caching the element arrays or computing counts in a single pass if this becomes a hot path.